Skip to content

Fix esify eslint#233

Merged
TylerHorth merged 1 commit intomasterfrom
fix-eslint
Nov 17, 2016
Merged

Fix esify eslint#233
TylerHorth merged 1 commit intomasterfrom
fix-eslint

Conversation

@TylerHorth
Copy link
Contributor

@TylerHorth TylerHorth commented Nov 17, 2016

Attempt to load local eslint and process with --fix.

Fixes #185

@lemonmade @GoodForOneFare

@lemonmade
Copy link
Member

Code looks good 👍

Can you check what the compile times before/ after are for a large-ish set of files (maybe try 10–20)? I want to make sure we don't regress too much on that measure.

@GoodForOneFare
Copy link
Member

🎩 went well, code's good. 🚢 assuming there isn't a significant perf regression.

@TylerHorth
Copy link
Contributor Author

TylerHorth commented Nov 17, 2016

I ran it on all coffeescript files in assets/javascripts/admin/modules/products/ (~30 files), there wasn't a large regression:

eslint real user sys
NO 34.53s 30.72s 22.85s
YES 38.34s 34.60s 23.01s

@lemonmade @GoodForOneFare 🚢 ?

@GoodForOneFare
Copy link
Member

So, like, ~125ms/file? For the use case of many people updating one file at a time, looks worth it to me.

@lemonmade
Copy link
Member

🐑

@TylerHorth TylerHorth merged commit f781204 into master Nov 17, 2016
@TylerHorth TylerHorth deleted the fix-eslint branch November 17, 2016 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESLint is not actually running

3 participants